-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
LCFS - Implement Email Notification Triggers in Backend for Subscribed Users #1226 #1402
Conversation
Backend Test Results493 tests ±0 491 ✅ ±0 1m 52s ⏱️ +4s For more details on these failures, see this check. Results for commit c77f4c6. ± Comparison against base commit 3c014c2. ♻️ This comment has been updated with latest results. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks really good, just one minor question. 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there a better place to keep this in the backend? Or is it used in enough places to justify having it in base?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Alex for the review. Since this schema will be used across various services, it causes greenlet spawn error if placed inside the notifications schema, hence it's being placed inside the base.
#1226